test: prune low-value tests and consolidate slow suites - #22
Conversation
…and rsc example Removes tests that re-assert coverage pinned elsewhere or assert file contents instead of behavior: the artifacts-real e2e file (its table coverage lives in overview), the 390px Runtime controls e2e, the safe launch configuration overview test (redaction lives in the epoch MCP session test), the handoff close-retry overview test (pinned by runtime-mcp-handoff.test.ts), config/source string-matching tests, and the rsc example's tsconfig/doc/manifest duplicates. Also trims the Inspector-tab detours from the Runtime sibling e2e ahead of the inspector removal.
…suites The Workbench is a desktop-only product validated at 1440x900, so the 390px viewport resizes and horizontal-overflow checks appended to real host e2e tests assert a layout the product does not ship. Removes only those mobile assertion sites, keeps every host test, runs the MCP App preview browser test at the desktop viewport, and deletes the mobile capture path (PNG, mobileLayout evidence, --mobile flag) from the runtime playground capture script plus the README/topology command strings that pin its exact invocation.
Each eviction test prepared its own dev session and drove fifty real invocations before exercising one hook, repeating the slowest setup in the suite four times (and flaking under load). One session now fills the fifty-artifact window once and drives the happy eviction, the held-reader reservation, the failed run-directory removal, and the failed artifact release in eviction order, preserving every distinct assertion including the readRunFlight path-traversal check and the close-retry accounting. Neighbouring retain-until-close, worker-bound, and containment tests are untouched.
…roduction build test The second multi-environment rebuild test repeated the full example build only to check that a planted stale async chunk disappears, and the self-contained widget HTML test duplicated assertions the host artifacts suite already makes per artifact. host-artifacts now plants the stale chunk before its existing production build and asserts its removal, and its HTML artifact loop keeps the inline script/style presence checks, so mcp-transports drops both duplicates.
…e unit pool The three finalization-polling tests exercise readFinalizedEvalRun with in-memory fake clients and never touch a browser or server, yet they lived in evals-real.e2e.test.ts and paid the serialized integration pool for it. They move verbatim to evals-finalized-run.test.ts, which the unit config picks up by default.
…nstead of the default integration pool Every npm pack + clean-install suite (dev-workbench-packaging, packed-consumer, packed-native-smoke, release-audit, rsc-runtime-optional-packaging) leaves the serialized integration pool and moves to a dedicated packedTestFiles list that the unit pool also excludes. The three pack+install cases in public-api.test.ts split into public-api-packed.test.ts so the cheap export and built-entrypoint checks stay in the default loop. test:packed now lists the split file plus rsc-runtime-optional-packaging and packed-native-smoke, keeping per-PR CI coverage through the release-gates job's check:release run.
…service tests Removes tests that only re-assert their own fixtures, string-match documentation, or duplicate a sibling suite: the public-api config/type re-export tests (the built-entrypoint test still pins subpath imports and defineConfig identity), the packed-consumer regex self-test, the manifest re-export identity test, the examples README string test, the topology capture-command README pin, the audiobook tool-catalog and CLI-receipt duplicates, the eval-cli duplicate of the eval-service run path, the release-audit pack dry run (check:release runs pack:dry-run directly), and one redundant JSON.stringify assertion in the native smoke. Type-only contracts previously wrapped in vacuous runtime tests (modern MCP transports, runtime provider binding/surface shapes) stay as module-level @ts-expect-error checks. Deletes the orphaned epoch-atomicity spike (production coverage lives in epoch-store and dev-lock tests) and the micro-eval spot-check suite together with its test:spot-check script and CI job; the examples:check step from that job survives as its own examples-check job.
…-metadata rehash test adapter-metadata.test.ts already rehashes every capability and schema snapshot against its pinned provenance for all built-in targets, so the host-adapters CLI-version test keeps only its unique assertions: the observed CLI version pins, the redacted help text, and the codex marketplace validator fixture.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: badb229009
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8243b59
into
fix/deflake-root-causes
- close the fixture-server listener before destroying held connections so a reconnecting browser cannot slip a new request past teardown (#24) - honor AGENT_BUNDLE_PACKAGE_PREBUILT in public-api.test.ts so the parallel integration pool never rewrites the shared dist directories (#27) - move packed-release.e2e into the packed suite so `pnpm test` and `check:release` stop running the same long packed-browser suite twice, retiring the now-empty serial integration pool (#22) - reconcile the fidelity ledger with the desktop-only capture interface and drop the retired mobile.png evidence (#22)
Summary
Applies the two reviewed test-audit hit lists on top of
fix/deflake-root-causes, one commit per phase.Deletions (vacuous / duplicate tests)
artifacts-real.e2e.test.tsandtsconfig-coverage.test.tsfiles.defineConfigidentity), the packed-consumer regex self-test, manifest re-export identities, README string tests, audiobook tool-catalog/CLI-receipt duplicates, the eval-cli duplicate of the eval-service path, the release-audit pack dry run (check:releaserunspack:dry-rundirectly), one redundant assertion in the native smoke, plus whole-file deletions of the orphanedepoch-atomicity-spike.test.tsandmicro-eval-spot-check.test.ts. Type-only contracts formerly wrapped in vacuous runtime tests survive as module-level@ts-expect-errorchecks.host-adapters.test.tsdrops the schema SHA-256 tables thatadapter-metadata.test.ts's rehash test already pins; version pins, help-text redaction, and the codex validator fixture stay.Mobile prunes (commit 2) — the Workbench is desktop-only (1440×900), so the 390px viewport-resize/overflow tails appended to nine e2e suites are gone, along with the entire mobile capture path in
capture-runtime-playground.mjs(PNG,mobileLayoutevidence,--mobileflag) and the README/topology command strings pinning it. Expected saving: ~2–3.5 min of Chrome e2e time.Consolidations
beforeRunArtifactRelease,beforeRunFlightRead,beforeRunDirectoryRemoval) in eviction order. Every distinct assertion is preserved, including thereadRunFlight('../flight.bin')path-traversal check and close-retry accounting. Expected saving: ~1.5–2.5 min and removes today's flake site.mcp-transports.integration.test.tsfold intohost-artifacts.test.ts's existing production-build test (stale chunk planted before the build, removal asserted after).Pool moves
readFinalizedEvalRunfake-client tests leaveevals-real.e2e.test.tsfor a new unit-pool fileevals-finalized-run.test.ts.packedTestFileslist (excluded from the unit pool too) and run viatest:packed/test:packed:native; the three pack+install cases inpublic-api.test.tssplit intopublic-api-packed.test.ts.test:packednow also listsrsc-runtime-optional-packagingandpacked-native-smokeso nothing loses CI coverage. Expected saving: ~5–9 min per default integration run.CI: the release-gates job already runs
check:release(→test:packed) on every PR/push, so pack coverage stays per-PR. The old "Micro-eval spot-check" job is replaced by anexamples-checkjob that keeps its survivingpnpm examples:checkstep; thetest:spot-checkscript and its README mentions are removed. Native packed smokes remain covered by the manually dispatched native-host-smoke workflow.Not touched, per the in-flight inspector refactor (#21):
sync-inspector.test.tsandrsbuild-closure.test.ts.Test plan
pnpm typecheckandpnpm lintafter every phasepnpm test:unit(full parallel pool, includes the newevals-finalized-run.test.ts)dev-invocation.integration.test.tsrun — consolidated eviction test passes with all neighborsrstest.config.ts:release-audit,packed-consumer,packed-native-smoke,public-api-packedhost-artifacts,mcp-transports.integration,docs-contract,runtime-artifact-manifestrstest testsrun